-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
overhaul exit codes for rustc and rustdoc #52197
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
I think if we make this change for |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
src/test/run-make/exit-code/foo.rs
Outdated
// except according to those terms. | ||
|
||
fn main() { | ||
compile_error!("kaboom"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not an ICE, so it's testing whether compilation failures produce 1 instead of 101. I thought the purpose was to make ices be 1 and errors stay 101.
You can test for ices by passing -Ztreat-err-as-bug
, which turns errors into ICEs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the purpose was to make ICEs stay as 101 (same as all panics) and make errors 1 (but see my comment below).
Thanks for the trick to create an ICE! I can make another test that ensures it outputs 101.
@Mark-Simulacrum I found another inconsistency: on invalid args rustc returns 101, while rustdoc returns 1. Let the bikeshed commence, but I propose the following for both rustc and rustdoc:
I'll also can add a note to the man pages. |
So after this we can remove rust/src/tools/compiletest/src/runtest.rs Line 1176 in 6cc42a4
And just check the exit code? |
src/librustc_driver/lib.rs
Outdated
@@ -1603,20 +1613,32 @@ fn extra_compiler_flags() -> Option<(Vec<String>, bool)> { | |||
} | |||
} | |||
|
|||
#[derive(Debug)] | |||
pub struct CompilationFailure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can probably make this just struct CompilationFailure;
4aaf158
to
91d6507
Compare
This commit changes the exit status of rustc to 1 in the presence of compilation errors. In the event of an unexpected panic (ICE) the standard panic error exit status of 101 remains. A run-make test is added to ensure that the exit code does not regress, and compiletest is updated to check for an exit status of 1 or 101, depending on the mode and suite. This is a breaking change for custom drivers. Fixes rust-lang#51971.
@Mark-Simulacrum @oli-obk ready for another review. Unlike my previous comment, I ended up setting the exit code for both bad args and compilation/lint failures to 1, because the error reporting machinery is the same for both. It would be significant effort to pull them apart, and doesn't seem necessary to tackle in this PR. I also ended up ignoring the ui/issue-20801.rs, because this PR uncovers that it ICEs with NLL enabled (#52416). Is there a way to ignore it solely for NLL? |
Ah whoops, I meant to ping @michaelwoerister. Sorry Mark! |
Looks good to me. I'll see if I can get rls and clippy lockstep upgraded so this PR doesn't break them. |
clippy and rls still test successfully on top of this PR. @bors r+ |
📌 Commit 8f4ccac has been approved by |
overhaul exit codes for rustc and rustdoc This commit changes the exit status of rustc to 1 in the presence of compilation errors. In the event of an unexpected panic (ICE) the standard panic error exit status of 101 remains. A run-make test is added to ensure that the exit code does not regress, and compiletest is updated to check for an exit status of 1 or 101, depending on the mode and suite. This is a breaking change for custom drivers. Note that while changes were made to the rustdoc binary, there is no intended behavior change. rustdoc errors (i.e., failed lints) will still report 101. While this could *also* hide potential ICEs, I will leave that work to a future PR. Fixes #51971.
☀️ Test successful - status-appveyor, status-travis |
Version 1.29.0 (2018-09-13) ========================== Compiler -------- - [Bumped minimum LLVM version to 5.0.][51899] - [Added `powerpc64le-unknown-linux-musl` target.][51619] - [Added `aarch64-unknown-hermit` and `x86_64-unknown-hermit` targets.][52861] Libraries --------- - [`Once::call_once` now no longer requires `Once` to be `'static`.][52239] - [`BuildHasherDefault` now implements `PartialEq` and `Eq`.][52402] - [`Box<CStr>`, `Box<OsStr>`, and `Box<Path>` now implement `Clone`.][51912] - [Implemented `PartialEq<&str>` for `OsString` and `PartialEq<OsString>` for `&str`.][51178] - [`Cell<T>` now allows `T` to be unsized.][50494] - [`SocketAddr` is now stable on Redox.][52656] Stabilized APIs --------------- - [`Arc::downcast`] - [`Iterator::flatten`] - [`Rc::downcast`] Cargo ----- - [Cargo can silently fix some bad lockfiles ][cargo/5831] You can use `--locked` to disable this behaviour. - [`cargo-install` will now allow you to cross compile an install using `--target`][cargo/5614] - [Added the `cargo-fix` subcommand to automatically move project code from 2015 edition to 2018.][cargo/5723] Misc ---- - [`rustdoc` now has the `--cap-lints` option which demotes all lints above the specified level to that level.][52354] For example `--cap-lints warn` will demote `deny` and `forbid` lints to `warn`. - [`rustc` and `rustdoc` will now have the exit code of `1` if compilation fails, and `101` if there is a panic.][52197] - [A preview of clippy has been made available through rustup.][51122] You can install the preview with `rustup component add clippy-preview` Compatibility Notes ------------------- - [`str::{slice_unchecked, slice_unchecked_mut}` are now deprecated.][51807] Use `str::get_unchecked(begin..end)` instead. - [`std::env::home_dir` is now deprecated for its unintuitive behaviour.][51656] Consider using the `home_dir` function from https://crates.io/crates/dirs instead. - [`rustc` will no longer silently ignore invalid data in target spec.][52330] [52861]: rust-lang/rust#52861 [52656]: rust-lang/rust#52656 [52239]: rust-lang/rust#52239 [52330]: rust-lang/rust#52330 [52354]: rust-lang/rust#52354 [52402]: rust-lang/rust#52402 [52103]: rust-lang/rust#52103 [52197]: rust-lang/rust#52197 [51807]: rust-lang/rust#51807 [51899]: rust-lang/rust#51899 [51912]: rust-lang/rust#51912 [51511]: rust-lang/rust#51511 [51619]: rust-lang/rust#51619 [51656]: rust-lang/rust#51656 [51178]: rust-lang/rust#51178 [51122]: rust-lang/rust#51122 [50494]: rust-lang/rust#50494 [cargo/5614]: rust-lang/cargo#5614 [cargo/5723]: rust-lang/cargo#5723 [cargo/5831]: rust-lang/cargo#5831 [`Arc::downcast`]: https://doc.rust-lang.org/std/sync/struct.Arc.html#method.downcast [`Iterator::flatten`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.flatten [`Rc::downcast`]: https://doc.rust-lang.org/std/rc/struct.Rc.html#method.downcast
Since rust-lang/rust#52197, ICEs will fail with exit status 101. Compilation successes and failures will return 0 and 1, respectively. Fixes rust-lang#143.
Since rust-lang/rust#52197, ICEs will fail with exit status 101. Compilation successes and failures will return 0 and 1, respectively. Fixes rust-lang#143.
This commit changes the exit status of rustc to 1 in the presence of
compilation errors. In the event of an unexpected panic (ICE) the
standard panic error exit status of 101 remains.
A run-make test is added to ensure that the exit code does not regress,
and compiletest is updated to check for an exit status of 1 or 101,
depending on the mode and suite.
This is a breaking change for custom drivers.
Note that while changes were made to the rustdoc binary, there is no
intended behavior change. rustdoc errors (i.e., failed lints) will still
report 101. While this could also hide potential ICEs, I will leave
that work to a future PR.
Fixes #51971.